Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Core] Adding SynchronizePointsWithBoundingBox method #11877

Merged
merged 15 commits into from
Dec 20, 2023

Conversation

loumalouomega
Copy link
Member

📝 Description

This method will be used in the future SearchWrapper class in order to estimate the points to be computed in the distributed search taking into account a bounding box filtering (only the points insise the BB).

🆕 Changelog

@loumalouomega loumalouomega added Enhancement Kratos Core Parallel-MPI Distributed memory parallelism for HPC / clusters Transition labels Dec 5, 2023
@loumalouomega loumalouomega requested a review from a team as a code owner December 5, 2023 14:12
@loumalouomega
Copy link
Member Author

@roigcarlo can you take a look?

kratos/utilities/search_utilities.h Outdated Show resolved Hide resolved
kratos/utilities/search_utilities.h Outdated Show resolved Hide resolved

// Auxiliary variables
std::size_t counter = 0;
array_1d<double, 3> coordinates;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a suggestion, I found the declaration of coordinates here confusing, because then you are forced to use noalias(coordinates) to avoid the copy. In this particular case, and since the only usage is to assign it, you could just

send_points_coordinates[3 * counter + i_coord] = it_point->Coordinates()[i_coord]

or

auto & coords = it_point->Coordinates();
send_points_coordinates[3 * counter + i_coord] = coords[i_coord]

But both of them would get optimized anyway....

kratos/utilities/search_utilities.h Show resolved Hide resolved
kratos/utilities/search_utilities.h Outdated Show resolved Hide resolved

// Some definitions
IndexType i_coord = 0;
array_1d<double, 3> coordinates;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similar comment, but in this function you are only using it in the serial condition so even better.

@loumalouomega
Copy link
Member Author

Shold be Okay now @roigcarlo

@loumalouomega loumalouomega merged commit 1614044 into master Dec 20, 2023
17 checks passed
@loumalouomega loumalouomega deleted the core/refactor-search-utilities-filter-bb branch December 20, 2023 17:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement Kratos Core Parallel-MPI Distributed memory parallelism for HPC / clusters Transition
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants